Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cwl): Add metric definitions for LiveTail #916

Merged
merged 4 commits into from
Nov 19, 2024

Conversation

keeganirby
Copy link
Contributor

Problem

Cloudwatch wants to monitor usage metrics of the LiveTail integration

Solution

When starting a session emit telemetry for:

  • Session already started (this case happens when session is already running, and customer sends a new command that matches the already running session)
  • Has LogEventFilter
  • LogStream filter type
  • Source of the command (command palette, explorer)

When closing a session:

  • Session duration
  • source of cancellation (ex: CodeLens, ClosingEditors)

License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@keeganirby keeganirby requested a review from a team as a code owner November 19, 2024 18:41
Copy link
Contributor

@jpinkney-aws jpinkney-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhruvigajjar dhruvigajjar merged commit a6aa0c8 into aws:main Nov 19, 2024
8 checks passed
@@ -1297,6 +1297,11 @@
"type": "boolean",
"description": "Whether the user has access to CodeWhisperer Chat"
},
{
"name": "hasLogEventFilterPattern",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is over-specific. Can you find a field name that will be useful for other metrics in the future, e.g. hasPatternFilter (why is "log event" important enough to call out in the field name?)

Another way of future-proofing is to use enum-like (or string) fields instead of booleans. It's pretty cumbersome that we have booleans for each of these variants, and may need to add even more in the future.

"name": "hasLogEventFilterPattern",
"type": "boolean",
"description": "If LogEvent filter pattern is applied"
},
{
"name": "hasTextFilter",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is hasLogEventFilterPattern different than this existing field?

@@ -4995,6 +5010,48 @@
}
]
},
{
"name": "cwlLiveTail_Start",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am certain that we have existing cloudwatch metrics. Yet these new "live tail" metrics don't use the same prefix, so now both will be harder to discover.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants